Nit: variable shadowing — The destructured [s, e] shadows the outer const s = new MagicString(code) on line 2947. This doesn't cause a functional bug (the arrow function only uses s for index comparison, not as a MagicString), but it's a readability hazard — a future reader might think s here refers to the MagicString.

Minor: ^ anchor with m flag can match inside template literals. A multiline template string like:

const x = `
import { Inter } from 'next/font/google'
`;

would be matched by this regex because the m flag makes ^ match at the start of any line, even inside template literals. In practice this is extremely unlikely to matter (who puts a next/font/google import inside a template string?), but worth noting if you want maximum robustness. The typical fix is to use an AST-based approach, but for a Vite transform this is a reasonable tradeoff.

Design note: self-hosting silently skipped for same-line import+call. When the import and constructor call are on the same line:

import { Inter } from 'next/font/google'; const inter = Inter({ weight: ['400'] });

The import rewriting works correctly (Inter gets replaced with createFontLoader call), but the self-hosting CSS injection is skipped because the call overlaps with the overwritten import region. The font will still work in production but will load from CDN instead of being self-hosted.

This is a fine tradeoff since the single-line pattern is rare, but a comment documenting this intentional limitation would help future readers understand why the overlap check exists and what the user-facing consequence is.

Good call on the IMPORTANT: keep this set in sync comment. For even more robustness, consider deriving this set programmatically from the actual exports of font-google.ts (e.g., in a test) rather than relying on a manual sync. A test like:

it('utilityExports matches actual non-font exports', async () => {
  const mod = await import('../packages/vinext/src/shims/font-google.js');
  const actual = Object.keys(mod).filter(k => k !== 'default');
  // Verify they match the hardcoded set in the plugin
});

would catch drift automatically.

Using path.resolve(__dirname, 'shims') is more robust than the previous id.includes('font-google') guard that was flagged by the earlier review. Good fix.

Good — the Proxy now handles both underscore-style names (Roboto_Mono) and PascalCase names (RobotoMono). This keeps the Proxy as a correct fallback for cases where the Vite plugin doesn't run (e.g., direct runtime access via mod.default.SomeFontName).

Good — this alias test was called out as missing in the previous review and has been added. Coverage for the import { X as Y } pattern is important since the rewriting code has separate imported vs local handling.

Good — test for fonts with digits after underscore (Baloo_2) was also called out as missing and has been added. The updated fontCallRe regex on line 2998 of index.ts now matches [A-Za-z0-9] after underscore, which handles this correctly.

Variable shadowing: [s, e] shadows the outer const s = new MagicString(code) from line 2947. This was flagged in the previous bonk review but hasn't been fixed. While it doesn't cause a functional bug today (the s in the arrow function is only used for index comparison), it's a readability hazard that could cause a real bug if someone later adds MagicString operations inside this callback.

Silent self-hosting skip deserves a comment: When the import and constructor call are on the same line (e.g., import { Inter } from 'next/font/google'; const inter = Inter({...});), the overlap check correctly prevents MagicString corruption by skipping the self-hosting injection. But the user-facing consequence — the font loads from CDN instead of being self-hosted — is non-obvious.

Consider adding a brief comment documenting this limitation:

Sync hazard: This hardcoded set must stay in sync with the actual exports from font-google.ts. The comment is helpful, but a test would be more reliable. Consider adding something like:

it('utilityExports matches actual non-font exports from font-google.ts', async () => {
  const mod = await import('../packages/vinext/src/shims/font-google.js');
  const expectedUtilities = Object.keys(mod).filter(k => k !== 'default' && typeof mod[k] === 'function');
  // compare against the hardcoded set in the plugin
});

This would catch drift automatically rather than relying on the comment.

Minor: The ^ anchor with m flag matches at the start of any line, including inside template literals. For example:

const x = `
import { Inter } from 'next/font/google'
`;

would be matched. In practice this is extremely unlikely (who puts a next/font/google import inside a template string?), so this is an acceptable tradeoff vs. a full AST parse. Just noting for awareness.

Nit: There's already a module-level constant _shimsDir at line 370 that computes the same path:

const _shimsDir = path.resolve(__dirname, "shims") + "/";

This line recomputes it on every transform call and uses path.sep instead of the hardcoded / that _shimsDir uses. On Windows, path.sep is \ while the existing constant uses /, so the behavior would differ.

Consider reusing _shimsDir for consistency and to avoid redundant computation:

The ^ anchor with m flag means this will match import at the start of any line, including inside multiline template literals or multiline comments. This has been noted in previous reviews as an acceptable tradeoff, and I agree — an AST-based approach would be overkill here.

However, there's one practical edge case worth considering: a multiline block comment like:

/*
import { Inter } from 'next/font/google'
*/

This would be matched and rewritten, breaking the comment. Consider whether adding a simple pre-check (e.g., stripping block comments first, or verifying the match isn't inside /* */) is worth the complexity. It's a judgment call — this pattern is unlikely but not impossible (e.g., someone commenting out an import during debugging).

Minor: The lambda parameter (s) shadows the outer MagicString variable s declared at line 2947. While not a bug (since s here is just the string from split), it's a readability hazard in a function that already had a variable shadowing issue flagged in previous reviews.

Consider renaming to avoid confusion:

When the first import is replaced with newImport + "\n" + fontDecls, the replacement text could be significantly longer or shorter than the original import line. This is fine for MagicString (it handles variable-length replacements), but it means the source map will map the entire block of const declarations back to a single original import line.

This is acceptable for a Vite transform — just noting it for awareness since debugging through source maps in this region may show unexpected line numbers.

Good that this is validated by the test at font-google.test.ts:672. One thing to watch: the getSSRFontPreloads export is in this set but isn't exercised in many of the plugin tests. If it were ever removed from font-google.ts, the sync test would catch it, but it's worth making sure the test at line 672 is robust (which it is — it compares sorted arrays).

Good improvement — the Proxy now handles both Roboto_Mono (underscore) and RobotoMono (PascalCase) naming conventions. This makes the fallback Proxy consistent with how the Vite plugin derives family names.

Note that the order matters here: replace(/_/g, " ") runs first, then replace(/([a-z])([A-Z])/g, ...). For a name like Roboto_Mono, the underscore replacement produces "Roboto Mono" and the PascalCase replacement is a no-op (since there's a space between o and M). For RobotoMono, the underscore replacement is a no-op and the PascalCase replacement produces "Roboto Mono". This is correct.

This is a valuable test — it catches drift between the hardcoded utilityExports set in the plugin and the actual exports from font-google.ts. Well done adding this after the review feedback.

One edge case: if someone adds a non-function export to font-google.ts (e.g., a constant or type re-export), this test would catch it since it checks all non-default keys. That's the right behavior.

The options-matching group \{[^}]*\} cannot match nested objects because [^}] stops at the first }. For example:

const inter = Inter({ weight: '400', axes: { wght: 400 } });

would not be matched by fontCallRe, so self-hosting would silently skip it (font falls back to CDN).

I realize this is a pre-existing limitation (the old code had the same regex), but since this PR is the natural place to improve it — and parseStaticObjectLiteral already handles nested objects correctly (there's even a test for it at line 611) — it's worth noting.

A simple brace-counting approach would fix this:

// Instead of \{[^}]*\}, match balanced braces
function extractBalancedBraces(code: string, openIndex: number): string | null {
  let depth = 0;
  for (let i = openIndex; i < code.length; i++) {
    if (code[i] === '{') depth++;
    else if (code[i] === '}') { depth--; if (depth === 0) return code.slice(openIndex, i + 1); }
  }
  return null;
}

Not blocking — just calling it out as a follow-up opportunity.

If a file has import { buildGoogleFontsUrl } from 'next/font/google' (only utility imports, no font names), the transform returns null and the import passes through unchanged. This is correct because font-google.ts already exports buildGoogleFontsUrl and the import will resolve normally.

However, if a file has import { type FontLoader } from 'next/font/google' (only type imports), fontImports is empty and we also return null. This is also correct since TypeScript will have already erased type-only imports before the Vite transform runs.

Just confirming I traced through both paths and they're handled correctly — nice.

Minor: createFontLoader as __vinext_clf is always added to newSpecifiers, even when it's already present in utilityImports (because the user explicitly imported createFontLoader). This would produce:

import { createFontLoader, createFontLoader as __vinext_clf } from 'next/font/google';

This is valid JS (duplicate specifiers are allowed), but it's slightly wasteful. Consider skipping the __vinext_clf alias when createFontLoader is already in utilityImports:

Alternatively, if createFontLoader is in utilityImports, you could remove it from there and only add the aliased version. Not blocking — the current output works.

The self-hosting regex runs on the original code while the MagicString s has already had import regions overwritten. This is correct because MagicString tracks overwrites by original-string indices, and the overlap check on line 3011 prevents double-overwrites. The s.overwrite(matchStart, matchEnd, ...) on line 3083 uses original-code indices, which is what MagicString expects.

This is a subtle but important correctness property — worth a one-line comment noting that fontCallRe intentionally runs on the original code (not s.toString()) because MagicString uses original indices.

The [ \t]* prefix allows optional indentation, but it also means the regex won't match an import preceded by, say, a semicolon on the same line:

;import { Inter } from 'next/font/google'

This is fine — that's not valid formatting. But there's one real-world edge case: a file with a BOM (byte order mark) at the very beginning. The BOM (\uFEFF) would appear before the first import and prevent the ^ anchor from matching on line 1 (since ^ with m flag matches start-of-line, and the BOM is a character before import).

This is extremely unlikely in practice (modern toolchains strip BOMs), so not blocking. Just noting for completeness.

This sync test is a great addition — it catches drift between the hardcoded utilityExports in the plugin and the actual exports from font-google.ts.

One subtle thing: the test imports from font-google.js and checks Object.keys(mod).filter(k => k !== 'default'). Since font-google.ts re-exports from font-google-base.ts, the keys here reflect the runtime exports. If someone added a type-only export to font-google.ts, it wouldn't show up in Object.keys(mod) and wouldn't trigger a test failure — but it also wouldn't need to be in utilityExports since the plugin only deals with runtime values. So this is correct.

The Proxy now handles both underscore (Roboto_Mono) and PascalCase (RobotoMono) naming. The order of replacements is correct: underscore-to-space first, then PascalCase-to-space. For Roboto_Mono, the first replacement produces "Roboto Mono" and the second is a no-op.

This makes the Proxy a correct fallback for any code path that bypasses the Vite plugin (e.g., direct mod.default.SomeFontName access at runtime).

When the first import is replaced with newImport + "\n" + fontDecls, the replacement text could be significantly longer than the original import line. This means the source map will map the entire block of const declarations back to a single original import line.

This is an acceptable tradeoff for a Vite transform, but it means that if a user sets a breakpoint on one of the generated const declarations, the debugger may behave unexpectedly. Worth knowing but not worth fixing — the hires: "boundary" option on line 3089 already provides the best mapping MagicString can offer for variable-length replacements.